-
Notifications
You must be signed in to change notification settings - Fork 39
refactor: Upgrade to Pydantic v2 #1566
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
859455c to
47951bb
Compare
47951bb to
03c9489
Compare
|
| Branch | refactor/pydantic-v2 |
| Testbed | ubuntu-22.04 |
🚨 1 Alert
| Benchmark | Measure Units | View | Benchmark Result (Result Δ%) | Lower Boundary (Limit %) |
|---|---|---|---|---|
| sync-v2 (up to 20000 blocks) | Latency minutes (m) | 📈 plot 🚷 threshold 🚨 alert (🔔) | 1.41 m(-18.01%)Baseline: 1.71 m | 1.54 m (109.76%) |
Click to view all benchmark results
| Benchmark | Latency | Benchmark Result minutes (m) (Result Δ%) | Lower Boundary minutes (m) (Limit %) | Upper Boundary minutes (m) (Limit %) |
|---|---|---|---|---|
| sync-v2 (up to 20000 blocks) | 📈 view plot 🚷 view threshold 🚨 view alert (🔔) | 1.41 m(-18.01%)Baseline: 1.71 m | 1.54 m (109.76%) | 2.06 m (68.33%) |
517d775 to
7250d52
Compare
| db_basic_settings = db_settings.copy(deep=True, exclude={'features'}) | ||
| new_basic_settings = new_settings.copy(deep=True, exclude={'features'}) | ||
| db_settings: FeatureActivationSettings = FeatureActivationSettings.model_validate_json(db_settings_bytes) | ||
| db_basic_settings = db_settings.model_copy(deep=True, update={'features': {}}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd rather use exclude instead of update.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems exclude is not available in Pydantic v2. I guess it is ok to simply replace it by an empty dict.
|
|
||
| class SideDagArgs(RunNodeArgs): | ||
| poa_signer_file: str | None | ||
| poa_signer_file: str | None = None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can it really be None?
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #1566 +/- ##
==========================================
- Coverage 85.80% 85.72% -0.08%
==========================================
Files 439 439
Lines 33758 33723 -35
Branches 5276 5277 +1
==========================================
- Hits 28966 28909 -57
- Misses 3784 3802 +18
- Partials 1008 1012 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
107d6c4 to
d0ac1fe
Compare
| token_name: Optional[str] = None | ||
| token_symbol: Optional[str] = None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pydantic v2 requires default value to ignore missing parameters. Should these None be included?
d0ac1fe to
84527e6
Compare
| class BaseEvent(BaseModel): | ||
| model_config = ConfigDict(use_enum_values=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Previous code set use_enum_values in addition to what's already configured on our BaseModel. Does this new syntax do that, too, instead of replacing all configs?
| for feature, criteria in self.features.items() | ||
| } | ||
| # Use object.__setattr__ because the model is frozen | ||
| object.__setattr__(self, 'features', validated_features) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's probably a better way to do this, instead of using __setattr__. Maybe it's a use case for mode='before'?
| @@ -1,4 +1,4 @@ | |||
| # This file is automatically @generated by Poetry 2.3.1 and should not be changed by hand. | |||
| # This file is automatically @generated by Poetry 2.1.3 and should not be changed by hand. | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use the most recent Poetry version
84527e6 to
eaa0320
Compare
eaa0320 to
cc58057
Compare
Motivation
This PR upgrades the codebase from Pydantic v1 to Pydantic v2.
Acceptance Criteria
Checklist
master, confirm this code is production-ready and can be included in future releases as soon as it gets merged